Skip to content

feat(analytics): event bridge common package#246

Closed
tylerjroach wants to merge 19 commits into
mainfrom
feat/event-bridge-common-package
Closed

feat(analytics): event bridge common package#246
tylerjroach wants to merge 19 commits into
mainfrom
feat/event-bridge-common-package

Conversation

@tylerjroach

Copy link
Copy Markdown
Contributor

No description provided.

tylerjroach and others added 4 commits May 21, 2026 14:03
Introduces packages/mixpanel_flutter_common (EventBridge + JSONLogic),
wires the native MixpanelEventBridge through to Dart on Android/iOS,
and adds forwarding tests. Held on a branch while the repo is
restructured into a packages/ monorepo layout; will be rebased onto
the new structure.
MethodChannel.invokeMethod on Android is thread-safe — no need to
occupy the UI thread for event fan-out.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The native MixpanelEventBridge collector (Kotlin SharedFlow on Android,
AsyncStream on iOS/macOS) previously spun up eagerly in the plugin's
register/attach lifecycle. Apps that never consume events on the Dart
side were paying for an idle MethodChannel round-trip on every tracked
event.

Now the Dart-side broadcast controller's onListen/onCancel drive
startEventBridge/stopEventBridge calls into native. The native
subscription only exists while at least one Dart consumer is attached
to MixpanelEventBridge.events, and is torn down when the last
listener cancels. The MethodCallHandler itself stays installed eagerly
so events can never race ahead of the dispatcher.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Introduces a new pure-Dart mixpanel_flutter_common package (EventBridge + JSONLogic), wires native→Dart event forwarding with lazy start/stop semantics, and adds extensive unit/fixture/security tests to keep behavior aligned across SDK ports.

Changes:

  • Added mixpanel_flutter_common package with MixpanelEventBridge, MixpanelEvent, and a JSONLogic parser/evaluator + exceptions.
  • Added broad automated test coverage: JSONLogic fixture runner, edge/security tests, and EventBridge behavior tests.
  • Updated mixpanel_flutter (Dart + Android + iOS/macOS) to forward native EventBridge events to Dart and to lazily activate native subscriptions via startEventBridge/stopEventBridge.

Reviewed changes

Copilot reviewed 24 out of 27 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
packages/mixpanel_flutter_common/test/jsonlogic/tests.json Adds the shared JSONLogic fixture corpus used by the Dart runner.
packages/mixpanel_flutter_common/test/jsonlogic/json_logic_test.dart Adds a parameterized test runner that executes tests.json fixtures.
packages/mixpanel_flutter_common/test/jsonlogic/json_logic_security_test.dart Adds DoS/defensive-limit tests (max depth, error truncation, allocation-free loops).
packages/mixpanel_flutter_common/test/jsonlogic/json_logic_extra_edge_case_test.dart Adds extra Dart-specific JSONLogic edge cases (numeric model, var path quirks, in semantics).
packages/mixpanel_flutter_common/test/jsonlogic/json_logic_edge_case_test.dart Adds parity tests for error paths and unsupported-operator allowlist.
packages/mixpanel_flutter_common/test/event_bridge_test.dart Adds unit tests for MixpanelEventBridge stream behavior and lifecycle callbacks.
packages/mixpanel_flutter_common/pubspec.yaml Defines the new mixpanel_flutter_common package metadata and constraints.
packages/mixpanel_flutter_common/pubspec.lock Locks resolved dev/test dependencies for the new package.
packages/mixpanel_flutter_common/lib/src/mixpanel_event.dart Introduces the MixpanelEvent value object.
packages/mixpanel_flutter_common/lib/src/jsonlogic/json_logic_rule.dart Adds typed JSONLogic rule AST classes.
packages/mixpanel_flutter_common/lib/src/jsonlogic/json_logic_parser.dart Adds JSONLogic parsing with supported-operator allowlist, depth bounding, and truncation.
packages/mixpanel_flutter_common/lib/src/jsonlogic/json_logic_exception.dart Adds JSONLogic exception hierarchy and user-facing messages.
packages/mixpanel_flutter_common/lib/src/jsonlogic/json_logic_evaluator.dart Adds JSONLogic evaluator with strict typing and O(1) loops for large operand lists.
packages/mixpanel_flutter_common/lib/src/event_bridge.dart Adds MixpanelEventBridge broadcast stream with lazy lifecycle hooks.
packages/mixpanel_flutter_common/lib/mixpanel_flutter_common.dart Exports mixpanel_flutter_common public API surface.
packages/mixpanel_flutter_common/analysis_options.yaml Enables recommended lints + strict casts/raw-type enforcement.
packages/mixpanel_flutter/test/event_bridge_forwarding_test.dart Adds Flutter-side test verifying native channel callback surfaces on MixpanelEventBridge.events.
packages/mixpanel_flutter/swift/Classes/SwiftMixpanelFlutterPlugin.swift Adds iOS/macOS lazy event bridge subscription + forwards events over MethodChannel.
packages/mixpanel_flutter/pubspec.yaml Adds dependency on mixpanel_flutter_common.
packages/mixpanel_flutter/pubspec.lock Locks the new path dependency on mixpanel_flutter_common.
packages/mixpanel_flutter/macos/mixpanel_flutter.podspec Adds explicit MixpanelSwiftCommon pod dependency for Swift import.
packages/mixpanel_flutter/lib/mixpanel_flutter.dart Wires reverse path (native→Dart event bridge) and adds start/stop lifecycle channel calls.
packages/mixpanel_flutter/ios/mixpanel_flutter.podspec Adds explicit MixpanelSwiftCommon pod dependency for Swift import.
packages/mixpanel_flutter/example/pubspec.lock Pulls mixpanel_flutter_common transitively for the example app.
packages/mixpanel_flutter/android/src/main/kotlin/com/mixpanel/mixpanel_flutter/EventBridgeSubscriber.kt Adds Android shared-flow subscriber forwarding events to Dart via MethodChannel.
packages/mixpanel_flutter/android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterPlugin.java Adds startEventBridge/stopEventBridge handlers and stops subscriber on detach.
packages/mixpanel_flutter/android/build.gradle Enables Kotlin + coroutines and adds required Mixpanel common dependency for EventBridge.

"malformed JSON object: '${_truncate(trimmed)}'",
);
}
if (decoded is! Map) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot claims value is List / value is Map / haystack is List will fail under strict-raw-types: true. Verified false — flutter analyze on the common package reports No issues found!. The rule flags raw types in declarations, not in is expressions. Type promotion from is List to List and then to List<Object?> works cleanly with strict-casts on too.

/// Parses any decoded JSON value into a [JsonLogicRule].
///
/// Internal use only - use [parse] for parsing JSON strings.
static JsonLogicRule parseValue(Object? value, [int depth = 0]) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment

Comment on lines +78 to +79
if (value is List) return _parseArray(value, depth);
if (value is Map) return _parseObject(value, depth);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment

);
}

static JsonLogicRule _parseObject(Map<Object?, Object?> obj, int depth) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment

Comment on lines +58 to +60
if (rule is ArrayRule) {
return rule.elements.map((e) => evaluate(e, data)).toList();
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment

// Data access helpers
// ===========================================================================

static Object? _evaluateVar(VarRule rule, Map<String, Object?> data) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment

Comment on lines +9 to +17
# Matches mixpanel_flutter's published SDK floor. JSONLogic is intentionally
# written without Dart 3 features (sealed classes, switch expressions,
# constructor tearoffs) so depending on this package doesn't force
# mixpanel_flutter consumers to bump their Dart version.
sdk: '>=2.12.0 <4.0.0'

dev_dependencies:
test: ^1.24.0
lints: ^4.0.0

@tylerjroach tylerjroach May 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a dev dependency and does not impact production. No concern here.

Comment on lines +419 to +421
// Force lazy initialization of the reverse-direction MethodCallHandler
// so any native events tracked after this point reach Dart subscribers.
_eventBridgeWired;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't warn.

Comment on lines +52 to +70
private func handleStartEventBridge(_ result: @escaping FlutterResult) {
guard eventBridgeTask == nil, let channel = channel else {
result(nil)
return
}
if #available(iOS 13.0, macOS 10.15, *) {
eventBridgeTask = Task {
for await event in MixpanelEventBridge.shared.eventStream() {
await MainActor.run {
channel.invokeMethod("onMixpanelEvent", arguments: [
"eventName": event.eventName,
"properties": event.properties,
])
}
}
}
}
result(nil)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need, we should be bumping to 13 minimum soon on iOS anyway. This is just a sanity check.

}
continue;
}
if (entry is! List || entry.length < 3) continue;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

….dart

Restores the file to its pre-branch formatting (Dart formatter pre-3.7
style) so the diff against main shows only the event-bridge wiring:
new import, _eventBridgeWired field + _wireEventBridge() method, and
the one-line _eventBridgeWired reference in init().
Previously, any entry in tests.json that wasn't a section-marker string
or a [rule, data, expected] triple was silently skipped, masking
fixture corruption and silently reducing coverage. Throw with the
offending index and value instead.
mixpanel-android-common:1.0.1 ships with Kotlin 2.0 metadata, which
the previous 1.9.0 toolchain can't parse — CI was failing with
"Class 'MixpanelEventBridge' was compiled with an incompatible
version of Kotlin. The binary version of its metadata is 2.0.0,
expected version is 1.8.0." Picking 2.1.0 also satisfies Flutter's
upcoming minimum-Kotlin warning.
Minimum version needed to read mixpanel-android-common:1.0.1's
Kotlin 2.0 metadata. Flutter's upcoming "at least 2.1.0"
recommendation can be addressed separately alongside the example
app's Kotlin bump.
The example app's root buildscript declares the KGP that wins on the
shared Gradle classpath, so its version governs how every module's
Kotlin code is compiled — including the plugin's. With the example
app still on 1.9.22, our plugin's Kotlin code was compiled by a 1.x
toolchain that can't read mixpanel-android-common's 2.0 metadata,
even though the plugin's own buildscript declares 2.0.0.
Even after bumping both the plugin's and example app's declared Kotlin
to 2.0.0, Flutter's gradle integration kept handing the plugin module
a Kotlin 1.9.x compiler, which refuses to read mixpanel-android-common's
2.0 metadata. The .class files are JVM-forward-compatible (the consumed
APIs are just a Flow and a data class), so adding
-Xskip-metadata-version-check to the plugin's kotlinOptions unblocks
compilation. This is the workaround the Kotlin compiler itself suggests
in the error message.
The version bumps in this branch's earlier commits (4193db7, af6d8c4,
6d20500) did not actually fix the CI failure — every iteration kept
producing the same "expected version is 1.8.0" metadata error,
because Flutter's plugin-loader picks the KGP for plugin subprojects
itself and ignores what each plugin's buildscript declares. The
metadata error is fully handled by -Xskip-metadata-version-check
in the plugin's kotlinOptions (commit b869ae5), which ships inside
our plugin and absorbs the mismatch for every consumer transparently.

Reverting these bumps keeps the example app representative of a
typical customer setup so we don't accidentally signal that
consumers need to bump their own Kotlin to use the SDK.
@tylerjroach tylerjroach marked this pull request as ready for review June 1, 2026 16:07
@tylerjroach tylerjroach requested review from a team and rahul-mixpanel June 1, 2026 16:07
rahul-mixpanel
rahul-mixpanel previously approved these changes Jun 2, 2026
rahul-mixpanel
rahul-mixpanel previously approved these changes Jun 3, 2026
tylerjroach and others added 6 commits June 3, 2026 16:16
The int-vs-int fast path matches mixpanel-swift-common's
JSONLogicEvaluator (which tries Int === Int before falling back to
Double coercion). mixpanel-android currently collapses everything to
double and loses precision above 2^53 — Flutter intentionally diverges
from Android here to match the more accurate iOS contract.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nt-precision ===

- event_bridge_forwarding_test: replace the misleadingly-named
  "unknown method names are ignored" test with one that asserts the
  null reply envelope (Flutter's wire-level signal for
  MissingPluginException). Add a test that confirms a throwing channel
  mock around start/stopEventBridge does not leak an uncaught async
  error into the surrounding zone — guards the .catchError on the
  lifecycle invokeMethod calls.
- json_logic_edge_case_test: cover the int-vs-int strict-equals fast
  path with four cases (distinct ints above 2^53, matching ints above
  2^53, mixed int/double of equal value, matching doubles) so the
  Flutter-vs-Android precision divergence stays intentional rather
  than drifting back via a future refactor.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@tylerjroach tylerjroach closed this Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants